Add support for timeouts to improve hard realtime operation #618
Conversation
|
Huh, that's new. But thanks for implementing it.. what is the size cost to that feature over here? Ughhhh @MX682X didn't we (mostly you) you just implement this in a way that doesn't match the api for dxcore didn't? |
|
... After looking at the source, I think I know how to keep a small version and add the setTimeout function. Thinking about it further, I don't see a need to change anything in the DxCore. I haven't changed anything in the API itself. The setTimout that people liked to use was from the print class. The timeout I've implemented has like 10-50ms until it fires (never measured). It will be enough for 98% of the users and is fairly small. Of course, when people start complaining about firing to slow or to fast, I would try to think about a solution with a user definable timeout. I'm kinda afraid to add a volatile 32bit variable that might end up using 200 bytes of flash in the two functions that need it. |
|
Dear god. I had a look at merging this 2.0.0... that is going to be a BFD It is not something I'm going to do without CI testing running on this repo so we can reports on the filesizes if nothing else. I am very concerned by the volume of code being added Also it doesn't look like any attention has been paid to the fact that this core has an option to disable millis timekeeping. Frankly, I don't think any timekeeping solution that relies on micros is appropriate in the Wire library, because that adds a requirement for not only micros being enabled (it often isn't, because people disable it to save flash) but also on interrupts being enabled.... The place in that code where it was doing the timeout in an ISR with a delay loop is IMO much more reasonable. You could get the delay loop by simply adding a counter to the polling loop. I'm fine with the timeout being an on/off thing with a fixed timeout like we did on megaTinyCore and DxCore and I'm not fine with pulling in the bloat of micros() just to use I2C if it's enabled at all on a core for parts with as little as 2k and 4k of flash. |
|
Gotcha - haven't had time to iterate on this but @MX682X's solution here seems more suited to this use case. Would you consider a counter-in-polling-loop version of this PR? It will be at least a few weeks until I get around to it, so I don't want to promise anything just yet or duplicate work if you're already on it! My first thought is to use a 16-bit counter, so some napkin math says that at 8MHz and perhaps ~15 instructions per iteration, average 2 IPC, that's 246ms max. |
|
Absolutely, I would love to see that as a PR One other thing though --- start from the version of Wire in the 2.0.0-dev branch. It has been changed significatly, and when I was merging it, there were a bunch of places where I was confused about what was being changed |
Most I2C master examples will hang the processor forever if an I2C slave device becomes unresponsive. As a result, the ArduinoCore-avr libraries recently added a number of new methods to the Wire library, to allow programs to recover from these conditions (e.g., by hard resetting the device).
This PR adds support for timeouts in the hardware-TWI and USI versions of ATTinyCore's Wire library.
Testing
I only tested this on an ATTiny84A, so only the USI part of it got tested, the hardware-TWI part could really use some more. I used the test sketch from arduino/reference-en#895:
To trigger a hang, I tied SCL low, preventing any device from raising the clock line - this represents a situation where the I2C device might be misbehaving. When I did, the timeout triggered, as expected, and the USI interface reset.